feat: emit $is_server property on captured events by turnipdabeets · Pull Request #643 · PostHog/posthog-python

turnipdabeets · 2026-06-02T19:37:24Z

Adds $is_server: true to every event captured by this SDK so PostHog ingestion can identify server-side events.

greptile-apps · 2026-06-02T19:40:38Z

Comments Outside Diff (1)

posthog/client.py, line 1319-1330 (link)

$is_server is set before the super_properties merge, so a caller who happens to include "$is_server" in their super_properties dict will silently override the SDK-set value. Since this property exists specifically to guarantee every event from this SDK is tagged as a server-side event, it should be set after the merge so it is never overridden. ($lib and $lib_version share the same gap, but $is_server is the property the ingestion pipeline relies on for this classification.)

Prompt To Fix With AI

This is a comment left during a code review.
Path: posthog/client.py
Line: 1319-1330

Comment:
`$is_server` is set before the `super_properties` merge, so a caller who happens to include `"$is_server"` in their `super_properties` dict will silently override the SDK-set value. Since this property exists specifically to guarantee every event from this SDK is tagged as a server-side event, it should be set after the merge so it is never overridden. (`$lib` and `$lib_version` share the same gap, but `$is_server` is the property the ingestion pipeline relies on for this classification.)



How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
posthog/client.py:1319-1330
`$is_server` is set before the `super_properties` merge, so a caller who happens to include `"$is_server"` in their `super_properties` dict will silently override the SDK-set value. Since this property exists specifically to guarantee every event from this SDK is tagged as a server-side event, it should be set after the merge so it is never overridden. (`$lib` and `$lib_version` share the same gap, but `$is_server` is the property the ingestion pipeline relies on for this classification.)

```suggestion
        msg["properties"]["$lib"] = "posthog-python"
        msg["properties"]["$lib_version"] = VERSION

        if disable_geoip is None:
            disable_geoip = self.disable_geoip

        if disable_geoip:
            msg["properties"]["$geoip_disable"] = True

        if self.super_properties:
            msg["properties"] = {**msg["properties"], **self.super_properties}

        msg["properties"]["$is_server"] = True
```

### Issue 2 of 2
posthog/test/test_client.py:162
**Missing test for super_properties override**

The test only verifies that `$is_server` is present in the happy-path `test_basic_capture`. Given the placement of the property before the `super_properties` merge, a test that constructs a client with `super_properties={"$is_server": False}` and asserts the outgoing event still has `$is_server == True` would catch the override issue and anchor the intended invariant.

_{Reviews (1): Last reviewed commit: "feat: emit $is_server property on captur..." | Re-trigger Greptile}

github-actions · 2026-06-02T19:41:19Z

posthog-python Compliance Report

Date: 2026-06-03 15:09:25 UTC
Duration: 176142ms

✅ All Tests Passed!

45/45 tests passed

Capture Tests

✅ 29/29 tests passed

View Details

Test	Status	Duration
Format Validation.Event Has Required Fields	✅	517ms
Format Validation.Event Has Uuid	✅	1508ms
Format Validation.Event Has Lib Properties	✅	1507ms
Format Validation.Distinct Id Is String	✅	1507ms
Format Validation.Token Is Present	✅	1507ms
Format Validation.Custom Properties Preserved	✅	1507ms
Format Validation.Event Has Timestamp	✅	1507ms
Retry Behavior.Retries On 503	✅	9516ms
Retry Behavior.Does Not Retry On 400	✅	3508ms
Retry Behavior.Does Not Retry On 401	✅	3509ms
Retry Behavior.Respects Retry After Header	✅	9510ms
Retry Behavior.Implements Backoff	✅	23533ms
Retry Behavior.Retries On 500	✅	7506ms
Retry Behavior.Retries On 502	✅	7512ms
Retry Behavior.Retries On 504	✅	7509ms
Retry Behavior.Max Retries Respected	✅	23531ms
Deduplication.Generates Unique Uuids	✅	1497ms
Deduplication.Preserves Uuid On Retry	✅	7515ms
Deduplication.Preserves Uuid And Timestamp On Retry	✅	14513ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry	✅	7512ms
Deduplication.No Duplicate Events In Batch	✅	1507ms
Deduplication.Different Events Have Different Uuids	✅	1507ms
Compression.Sends Gzip When Enabled	✅	1507ms
Batch Format.Uses Proper Batch Structure	✅	1507ms
Batch Format.Flush With No Events Sends Nothing	✅	1005ms
Batch Format.Multiple Events Batched Together	✅	1505ms
Error Handling.Does Not Retry On 403	✅	3509ms
Error Handling.Does Not Retry On 413	✅	3508ms
Error Handling.Retries On 408	✅	7514ms

Feature_Flags Tests

✅ 16/16 tests passed

View Details

Test	Status	Duration
Request Payload.Request With Person Properties Device Id	✅	1002ms
Request Payload.Flags Request Uses V2 Query Param	✅	1007ms
Request Payload.Flags Request Hits Flags Path Not Decide	✅	1006ms
Request Payload.Flags Request Omits Authorization Header	✅	1007ms
Request Payload.Token In Flags Body Matches Init	✅	1007ms
Request Payload.Groups Round Trip	✅	1007ms
Request Payload.Groups Default To Empty Object	✅	1007ms
Request Payload.Person Properties Distinct Id Auto Populated When Caller Omits It	✅	1006ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False	✅	1007ms
Request Payload.Disable Geoip Omitted Defaults To False	✅	1007ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key	✅	1006ms
Request Lifecycle.No Flags Request On Init Alone	✅	504ms
Request Lifecycle.No Flags Request On Normal Capture	✅	1507ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests	✅	1010ms
Request Lifecycle.Mock Response Value Is Returned To Caller	✅	1003ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event	✅	1509ms

…ridden

ioannisj

Approving to unblock, left a couple of comments though

ioannisj · 2026-06-03T06:32:26Z

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch


This should probably be minor since we are adding a new constructor param?

Your call here but if we change we'll need to update all other SDKs that provide a config option as well

update all to use minor

ioannisj · 2026-06-03T06:34:20Z

+        if self.is_server:
+            msg["properties"]["$is_server"] = True


Curious, why do we omit vs set to False?

makes sense to me, we dont send on the client side so its the same semantics

ioannisj · 2026-06-03T06:35:52Z

 project_api_key = None  # type: Optional[str]
 poll_interval = 30  # type: int
 disable_geoip = True  # type: bool
+is_server = True  # type: bool


We are missing a docstring entry above

updated, should be there for all others as well

ioannisj · 2026-06-03T07:05:01Z

 project_api_key = None  # type: Optional[str]
 poll_interval = 30  # type: int
 disable_geoip = True  # type: bool
+is_server = True  # type: bool


Bool would be hard to change in the future without breaking changes if needed. Wondering if we should be defining as String (server, client for now) which will give us flexibility for the future. Based on the comment here for example this could also be "cli" (we'd need to rename the property of course to maybe something like $runtime_type)

I'll hold back review on other SDKs since they follow the same patters so these points can be discussed here?

CLI is also a client side, its all about if its running in the users machine or not at the end of the day, does not matter if its a desktop app, a mobile app, a CLI, etc
its either local or cloud, not sure if we need more than a boolean but good point

yea I think bool is ok, opening up to strings can make it complicated and we really just want to know if its server or not.

feat: emit $is_server property on captured events

cd1db41

turnipdabeets requested a review from a team as a code owner June 2, 2026 19:37

greptile-apps Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread posthog/test/test_client.py

turnipdabeets added 4 commits June 2, 2026 15:49

chore: add changeset for $is_server property

c2be91e

feat: make $is_server configurable (default true)

2f1a29d

fix: set $is_server after super_properties merge so it cannot be over…

214b79f

…ridden

test: include $is_server in group_identify exact-match assertions

2037d02

ioannisj approved these changes Jun 3, 2026

View reviewed changes

ioannisj reviewed Jun 3, 2026

View reviewed changes

turnipdabeets added 2 commits June 3, 2026 10:52

chore: bump changeset to minor (new is_server option)

b9760e7

docs: document is_server option

4e9fc63

turnipdabeets merged commit 3aed638 into main Jun 3, 2026
28 checks passed

turnipdabeets deleted the feat/add-is-server-property branch June 3, 2026 15:12

Conversation

turnipdabeets commented Jun 2, 2026

Uh oh!

greptile-apps Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comments Outside Diff (1)

Uh oh!

Uh oh!

github-actions Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

posthog-python Compliance Report

✅ All Tests Passed!

Capture Tests

Feature_Flags Tests

Uh oh!

ioannisj left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

greptile-apps Bot commented Jun 2, 2026 •

edited

Loading

github-actions Bot commented Jun 2, 2026 •

edited

Loading